Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugins/bcli: use -rpcwait to simplify waiting for bitcoind to warm up #7967

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NishantBansal2003
Copy link

Replaced custom wait logic with the -rpcwait flag in bitcoin-cli to handle waiting for bitcoind to warm up. Added a timer to print the warm-up message only if bitcoind doesn't start serving RPC calls within 30 seconds.
This simplifies the code and ensures that errors unrelated to warmup are passed up directly without additional checks.

Fixes: #3505

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! 🧡 The timeout needs work though...

plugins/bcli.c Outdated
tal_free(output);
struct timers *timer = tal(cmd, struct timers);
timers_init(timer, time_mono());
new_reltimer(timer, timer, time_from_sec(BITCOIND_WARMUP_TIMER),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work, unfortunately. We'll be stuck in the waitpid call, not in the ioloop.

But the correct way to do this is to also use rpcwaittimeout, so it will fail, and we can log a message and retry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(note: rpcwait was supported since bitcoin 0.9, and rpcwaittimeout since 22.0, so we're good here)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my understanding, does the timer run synchronously with the event loop, blocking the program until it expires? Or does it run in the background (in a non-blocking manner), calling the function asynchronously, if timer expires or get cleaned up by tal_free?

Replaced custom wait logic with the -rpcwait flag in bitcoin-cli to handle waiting for bitcoind to warm up. This simplifies the code and ensures that errors unrelated to warmup are passed up directly without additional checks.
Changelog-None

Signed-off-by: Nishant Bansal <[email protected]>
@NishantBansal2003
Copy link
Author

Hey @rustyrussell, just checking in to see if there’s any further feedback on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cleanup: Use the -rpcwait when waiting for bitcoind to warm up
2 participants